pkg/retry/retry_test.go (1)

68-70: ⚡ Quick win

Redundant error unwrapping: use either errors.Cause or errors.Is, not both.

errors.Cause from pingcap/errors already unwraps to the root error, so applying stderrors.Is on top is redundant. Choose one approach:

• Option 1: case errors.Cause(err) == context.Canceled:
• Option 2: case errors.Is(err, context.Canceled):

♻️ Proposed refactor (Option 1)

 	err := Do(context.Background(), f, WithMaxTries(3), WithIsRetryableErr(func(err error) bool {
 		switch {
-		case stderrors.Is(errors.Cause(err), context.Canceled):
+		case errors.Cause(err) == context.Canceled:
 			return false
 		}
 		return true

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/retry/retry_test.go` around lines 68 - 70, The switch block currently
uses redundant unwrapping (stderrors.Is(errors.Cause(err), context.Canceled));
replace that with a single check using the standard library: change to `case
errors.Is(err, context.Canceled):` (or alternatively `case errors.Cause(err) ==
context.Canceled:` if you prefer pingcap/errors equality), updating imports if
needed so only the chosen form remains.

pkg/sink/mysql/mysql_writer.go (1)

264-264: ⚡ Quick win

Redundant error unwrapping: use either errors.Cause or errors.Is, not both.

errors.Cause from pingcap/errors already unwraps to the root error, so applying stderrors.Is on top is redundant. Choose one approach:

• Option 1: errors.Cause(err) == cerror.ErrMySQLDuplicateEntry
• Option 2: errors.Is(err, cerror.ErrMySQLDuplicateEntry) (using pingcap/errors.Is if available, or stdlib)

♻️ Proposed refactor (Option 1)

-	if stderrors.Is(errors.Cause(err), cerror.ErrMySQLDuplicateEntry) ||
+	if errors.Cause(err) == cerror.ErrMySQLDuplicateEntry ||

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/sink/mysql/mysql_writer.go` at line 264, The conditional is redundantly
unwrapping the error by calling errors.Cause(err) and then passing that to
stderrors.Is; replace with a single check using one approach: either compare the
cause directly (errors.Cause(err) == cerror.ErrMySQLDuplicateEntry) or use
errors.Is on the original error (stderrors.Is(err,
cerror.ErrMySQLDuplicateEntry) or pingcap/errors.Is if available). Update the
conditional that currently uses errors.Cause(err) and stderrors.Is to use one of
these options consistently (refer to errors.Cause, stderrors.Is, and
cerror.ErrMySQLDuplicateEntry in mysql_writer.go) and remove the redundant
unwrap.

pkg/orchestrator/etcd_worker_test.go (1)

276-277: ⚡ Quick win

Redundant error unwrapping: use either errors.Cause or errors.Is, not both.

errors.Cause from pingcap/errors already unwraps to the root error, so applying stderrors.Is on top is redundant. Choose one approach:

• Option 1: errors.Cause(err) == context.DeadlineExceeded
• Option 2: errors.Is(err, context.DeadlineExceeded)

♻️ Proposed refactor (Option 1)

-	if err != nil && (stderrors.Is(errors.Cause(err), context.DeadlineExceeded) ||
-		stderrors.Is(errors.Cause(err), context.Canceled) ||
+	if err != nil && (errors.Cause(err) == context.DeadlineExceeded ||
+		errors.Cause(err) == context.Canceled ||

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/orchestrator/etcd_worker_test.go` around lines 276 - 277, The condition
is redundantly unwrapping errors by calling errors.Cause(err) and then
stderrors.Is; replace that pattern with a single standard check using
stderrors.Is(err, context.DeadlineExceeded) and stderrors.Is(err,
context.Canceled) (or alternatively use errors.Cause(err) ==
context.DeadlineExceeded if you prefer exact root comparison), i.e., update the
conditional that references errors.Cause(err) and stderrors.Is so it directly
calls stderrors.Is with err for both DeadlineExceeded and Canceled checks.

pkg/sink/codec/csv/csv_decoder.go (1)

113-113: ⚡ Quick win

Redundant error unwrapping: use either errors.Cause or errors.Is, not both.

errors.Cause from pingcap/errors already unwraps to the root error, so applying errors.Is on top is redundant. Choose one approach:

• Option 1: errors.Cause(err) == io.EOF
• Option 2: errors.Is(err, io.EOF)

♻️ Proposed refactor (Option 1)

-		if errors.Is(errors.Cause(err), io.EOF) {
+		if errors.Cause(err) == io.EOF {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/sink/codec/csv/csv_decoder.go` at line 113, Replace the redundant
unwrapping in csv_decoder.go where you currently call
errors.Is(errors.Cause(err), io.EOF); use one approach — prefer idiomatic
errors.Is — so change the check to errors.Is(err, io.EOF). Update the
conditional that inspects the variable err (the EOF check) to call
errors.Is(err, io.EOF) and remove the use of errors.Cause.

pkg/upstream/upstream.go (1)

140-146: ⚡ Quick win

Consider simplifying the single-case switch to an if statement.

A switch with only one case is less idiomatic than a simple if statement.

♻️ Proposed refactor

 func isCreateTiStoreRetryable(ctx context.Context, err error) bool {
-	switch {
-	case errors.Is(errors.Cause(err), context.Canceled):
+	if stderrors.Is(errors.Cause(err), context.Canceled) {
 		return ctx.Err() == nil
 	}
 	return true
 }

Note: You'll also need to ensure stderrors is imported as shown in other functions in this PR.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/upstream/upstream.go` around lines 140 - 146, Simplify the single-case
switch in isCreateTiStoreRetryable by replacing it with an if: check if
errors.Is(errors.Cause(err), context.Canceled) and if so return ctx.Err() ==
nil, otherwise return true; also ensure the same error package alias used
elsewhere in the PR (stderrors) is imported/used consistently for
errors.Cause/Is so imports compile.

pkg/pdutil/api_client.go (1)

190-196: ⚡ Quick win

Consider simplifying the single-case switch to an if statement.

A switch with only one case is less idiomatic than a simple if statement. The same pattern appears at lines 367-373.

♻️ Proposed refactor

-		retry.WithIsRetryableErr(func(err error) bool {
-			switch {
-			case stderrors.Is(errors.Cause(err), context.Canceled):
-				return false
-			}
-			return true
-		}))
+		retry.WithIsRetryableErr(func(err error) bool {
+			if stderrors.Is(errors.Cause(err), context.Canceled) {
+				return false
+			}
+			return true
+		}))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/pdutil/api_client.go` around lines 190 - 196, Replace the single-case
switch used inside the retry.WithIsRetryableErr lambda with a simple if
statement to make the code more idiomatic: in the anonymous function passed to
retry.WithIsRetryableErr, check if errors.Cause(err) is context.Canceled using
an if and return false in that branch, otherwise return true; apply the same
change to the similar occurrence around lines 367-373 so both usages of
retry.WithIsRetryableErr use an if instead of a switch.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tests/integration_tests/util/db.go (1)

156-158: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Impossible && condition still suppresses real exec failures.

The matcher was switched from == to cerror.Is(...) (likely to satisfy errorlint), but the underlying logic bug is unchanged: a single error cannot be both context.DeadlineExceeded and context.Canceled, so the predicate is always false and log.S().Fatal(err) is dead. Any non-context error from conn.ExecContext is silently swallowed.

The intent is to fatal on unexpected errors and tolerate context cancellation/deadline, i.e., invert the checks and join with &&:

Proposed fix

-	if err != nil && cerror.Is(errors.Cause(err), context.DeadlineExceeded) && cerror.Is(errors.Cause(err), context.Canceled) {
-		log.S().Fatal(err)
-	}
+	if err != nil &&
+		!cerror.Is(errors.Cause(err), context.DeadlineExceeded) &&
+		!cerror.Is(errors.Cause(err), context.Canceled) {
+		log.S().Fatal(err)
+	}

Also worth confirming that cerror.Is (from github.com/pingcap/ticdc/pkg/errors) correctly matches stdlib sentinel errors like context.DeadlineExceeded / context.Canceled after errors.Cause unwrapping — if it's not a transparent wrapper over stderrors.Is, this may not behave as == did previously.

#!/bin/bash
# Verify how cerror.Is is implemented in pingcap/ticdc/pkg/errors,
# to confirm it works with stdlib sentinel errors like context.DeadlineExceeded.
fd -t f -e go . pkg/errors | head -50
ast-grep --pattern 'func Is($_, $_) $_ {
  $$$
}'
rg -nP --type=go -C3 '^\s*(func|var)\s+Is\b' -g 'pkg/errors/**'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration_tests/util/db.go` around lines 156 - 158, The condition
currently uses cerror.Is(errors.Cause(err), context.DeadlineExceeded) &&
cerror.Is(errors.Cause(err), context.Canceled) which is always false and hides
real errors; change the logic to fatal on any non-nil err except when the cause
is context.DeadlineExceeded or context.Canceled, e.g. if err != nil && !
(cerror.Is(errors.Cause(err), context.DeadlineExceeded) ||
cerror.Is(errors.Cause(err), context.Canceled)) { log.S().Fatal(err) }; ensure
you update the conditional around the conn.ExecContext error handling and keep
errors.Cause and cerror.Is usage, and optionally add a quick check that
cerror.Is in pkg/errors correctly matches stdlib sentinel errors after
unwrapping.

pkg/etcd/client.go (1)

143-143: 💤 Low value

Consider simplifying error unwrapping.

The combination of errors.Cause with cerror.Is is redundant since cerror.Is already unwraps error chains internally (similar to the standard errors.Is).

♻️ Proposed simplification

-		if err != nil && !cerror.Is(errors.Cause(err), context.Canceled) {
+		if err != nil && !cerror.Is(err, context.Canceled) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/etcd/client.go` at line 143, The conditional uses errors.Cause together
with cerror.Is redundantly; change the check to let cerror.Is handle unwrapping
directly (i.e., replace the use of errors.Cause(err) with err) so the condition
becomes: if err != nil && !cerror.Is(err, context.Canceled); update the
occurrence in pkg/etcd/client.go where the variable err is tested with cerror.Is
and errors.Cause to remove errors.Cause and pass err directly.

logservice/logpuller/region_request_worker.go (1)

108-108: 💤 Low value

Simplify the wrapped-error check pattern.

The pattern cerror.Is(errors.Cause(err), context.Canceled) is redundant because errors.Cause() unwraps to the root error, then cerror.Is() walks the error chain again. Since cerror.Is() already traverses the entire error chain, you can simplify this to cerror.Is(err, context.Canceled) directly.

This same redundancy appears in multiple files across this PR (e.g., pkg/orchestrator/etcd_worker_test.go:275-276, pkg/workerpool/async_pool_test.go:128, pkg/sink/mysql/mysql_writer.go:263, pkg/logger/log.go:338), while pkg/messaging/message_center.go:476 correctly uses pkgerror.Is(err, ...) without errors.Cause(). Consider applying the simpler pattern consistently throughout.

♻️ Proposed refactor

-			if cerror.Is(errors.Cause(err), context.Canceled) {
+			if cerror.Is(err, context.Canceled) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@logservice/logpuller/region_request_worker.go` at line 108, Replace redundant
wrapped-error checks that call errors.Cause before cerror.Is with a direct
cerror.Is(err, context.Canceled) call; locate instances like the check in
region_request_worker.go (the if using cerror.Is(errors.Cause(err),
context.Canceled)) and change them to cerror.Is(err, context.Canceled). Apply
the same simplification to other occurrences (e.g., tests and writers) so all
checks use cerror.Is directly on the original error value rather than
errors.Cause(err).